feat: Enhance QueryBuilder with DELETE and MERGE query support #214
Conversation
- Added methods to build DELETE and MERGE queries in QueryBuilder. - Implemented normalization functions for DELETE and MERGE options. - Updated existing methods to hoist CTEs and handle explicit columns. - Introduced tests for buildDeleteQuery, buildInsertQuery, and buildMergeQuery to ensure correctness. - Refactored existing update and insert query methods to improve column handling and error messaging.
WalkthroughAdds conversion APIs to transform SELECT queries into INSERT, UPDATE, DELETE, and MERGE queries, extends QueryBuilder with new buildXQuery methods, introduces an InsertQuery <-> VALUES transformer, and adds tests exercising conversions and error cases. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev
participant SelectQ as SelectQuery\n(Simple/Binary/Values)
participant QB as QueryBuilder
participant Conv as InsertQuerySelectValuesConverter
participant Result as Result Query
Dev->>SelectQ: call toInsert/Update/Delete/Merge(options)
SelectQ->>QB: buildInsert/Update/Delete/Merge(select, options)
QB->>QB: normalize options\ninfer & validate columns
QB->>Result: assemble target query\n(Insert/Update/Delete/Merge)
Result-->>Dev: return built query
Note over Conv,QB: Converter used for VALUES ↔ SELECT\ntransformations when invoked explicitly
Dev->>Conv: toSelectUnion / toValues(insertQuery)
Conv->>Conv: validate shape, flatten selects,\nmap columns↔tuples
Conv-->>Dev: return transformed InsertQuery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (16)
packages/core/tests/transformers/buildInsertQuery.test.ts (1)
7-7: Add one more negative test for wildcard/unnamed columns.Add a case like
SELECT * FROM torSELECT 1(no alias) to assert the error: “Columns cannot be inferred from the selectQuery...”. This guards regressions in column inference. Based on learnings.packages/core/tests/transformers/buildDeleteQuery.test.ts (1)
7-7: Suggest a couple of extra cases for resilience.
- CTE hoisting:
WITH c AS (...) SELECT ...→ ensure WITH is promoted above DELETE.- Composite PKs and duplicate column names in
columns: verify de-duplication and predicate order.- Omit
sourceAliasto verify default'src'.packages/core/tests/transformers/buildUpdateQuery.test.ts (1)
7-7: Add one more negative test and a tiny cleanup.
- Negative: when the SELECT only contains primary keys (no non-PK columns), expect “No updatable columns found...”. This locks the boundary.
- Optional: extract a small
formatSql(q)helper to reduce repetition.packages/core/tests/transformers/buildMergeQuery.test.ts (2)
8-8: Optionally add one end-to-end SQL assertion.Add a single case that formats a MERGE to SQL via SqlFormatter and asserts the string. It complements the AST checks and catches formatter regressions. Based on learnings.
113-124: Future polish: clarify the insert‑columns error message.When
insertColumnsare provided but none match the SELECT output, the current error reads as if columns weren’t provided. Consider a distinct message (e.g., “Provided insertColumns do not exist in selectQuery output”). Not a blocker; test can remain as is.packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (1)
8-17: Add two more negative cases for stricter guarantees.
- SELECT→VALUES with FROM or WHERE should throw (guard already exists).
- VALUES→SELECT with a tuple whose length doesn’t match the column list should throw.
packages/core/src/models/SimpleSelectQuery.ts (1)
133-147: Document side effects of conversion methods (they may reorder/mutate selectClause)QueryBuilder’s builders can mutate the current SimpleSelectQuery (e.g., column reordering). Add a brief note to each toXQuery JSDoc, or consider invoking the builders on a defensive clone to avoid surprising changes to
this.packages/core/src/models/ValuesQuery.ts (1)
1-19: Remove unused import
ParameterCollectorisn’t used in this file; drop it to keep imports clean.-import { ParameterCollector } from "../transformers/ParameterCollector";packages/core/src/models/BinarySelectQuery.ts (2)
2-9: Drop unused type import
CTEOptionsisn’t used here.-import type { - SelectQuery, - CTEOptions, - InsertQueryConversionOptions, - UpdateQueryConversionOptions, - DeleteQueryConversionOptions, - MergeQueryConversionOptions -} from "./SelectQuery"; +import type { + SelectQuery, + InsertQueryConversionOptions, + UpdateQueryConversionOptions, + DeleteQueryConversionOptions, + MergeQueryConversionOptions +} from "./SelectQuery";
14-14: Remove unused import
ParameterCollectoris not referenced.-import { ParameterCollector } from "../transformers/ParameterCollector";packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (3)
1-7: Prefer type-only import and direct model imports to avoid cyclesUse a type-only import for
SelectQuery, and importSimpleSelectQuery/BinarySelectQueryfrom their modules to reduce barrel-induced cycles.-import { SimpleSelectQuery, BinarySelectQuery } from "../models/SelectQuery"; +import { SimpleSelectQuery } from "../models/SimpleSelectQuery"; +import { BinarySelectQuery } from "../models/BinarySelectQuery"; -import { SelectQuery } from "../models/SelectQuery"; +import type { SelectQuery } from "../models/SelectQuery";
79-102: Strengthen SELECT→VALUES validation and alias handling
- Reject extra clauses beyond FROM/WHERE (e.g., GROUP BY, HAVING, WINDOW, ORDER, LIMIT/OFFSET, FETCH, FOR).
- Detect duplicate aliases; require a 1:1 mapping to
columnNames.- const tuples = simpleQueries.map(query => { - if (query.fromClause || (query.whereClause && query.whereClause.condition)) { - throw new Error("SELECT queries with FROM or WHERE clauses cannot be converted to VALUES."); - } + const tuples = simpleQueries.map(query => { + const hasProhibitedClauses = + !!query.fromClause || + !!query.whereClause || + !!query.groupByClause || + !!query.havingClause || + !!query.windowClause || + !!query.orderByClause || + !!query.limitClause || + !!query.offsetClause || + !!query.fetchClause || + !!query.forClause; + if (hasProhibitedClauses) { + throw new Error("SELECT queries with FROM/WHERE/GROUP/HAVING/WINDOW/ORDER/LIMIT/OFFSET/FETCH/FOR cannot be converted to VALUES."); + } const valueMap = new Map<string, ValueComponent>(); for (const item of query.selectClause.items) { const identifier = item.identifier?.name ?? null; if (!identifier) { throw new Error("Each SELECT item must have an alias matching target columns."); } - if (!valueMap.has(identifier)) { - valueMap.set(identifier, item.value); - } + if (valueMap.has(identifier)) { + throw new Error(`Duplicate SELECT item alias '${identifier}'.`); + } + valueMap.set(identifier, item.value); } + if (valueMap.size !== columnNames.length) { + throw new Error("SELECT item aliases must match the target column list 1:1."); + }
17-24: Clarify error when selectQuery is null vs not a VALUES queryIf
insertQuery.selectQueryis null, the current message (“not a VALUES query”) can mislead. Consider a more precise error for null/undefined.- if (!(valuesQuery instanceof ValuesQuery)) { - throw new Error("InsertQuery selectQuery is not a VALUES query."); - } + if (!valuesQuery) { + throw new Error("InsertQuery has no selectQuery to convert."); + } + if (!(valuesQuery instanceof ValuesQuery)) { + throw new Error("InsertQuery.selectQuery is not a VALUES query."); + }packages/core/src/transformers/QueryBuilder.ts (3)
40-46: Normalize after building the full binary tree (CTE hoisting completeness).normalize is called before appending later queries, so CTEs introduced by subsequent queries may be missed. Normalize once after the loop.
Apply this diff:
- CTENormalizer.normalize(result); @@ - for (let i = 2; i < queries.length; i++) { + for (let i = 2; i < queries.length; i++) { result.appendSelectQuery(operator, wrap(queries[i])); } - return result; + // Normalize after the full tree is built (captures CTEs from all branches). + result = CTENormalizer.normalize(result) as BinarySelectQuery; + return result;
253-255: Guard against alias collision (target vs source) and clarify the error message.If sourceAlias equals target alias/name, generated predicates and column refs become ambiguous. Also, the Update error message mentions alias even though a table name fallback is accepted.
Apply this diff:
@@ - if (!targetAlias) { - throw new Error(`Source expression does not have an alias. Please provide an alias for the source expression.`); - } + if (!targetAlias) { + throw new Error(`Unable to resolve target alias/name from source expression.`); + } + if (targetAlias === options.sourceAlias) { + throw new Error(`sourceAlias ('${options.sourceAlias}') must differ from target alias/name ('${targetAlias}').`); + } @@ - if (!targetAlias) { - throw new Error(`Source expression does not have an alias. Please provide an alias for the delete target.`); - } + if (!targetAlias) { + throw new Error(`Source expression must have an explicit alias for DELETE USING.`); + } + if (targetAlias === normalized.sourceAlias) { + throw new Error(`sourceAlias ('${normalized.sourceAlias}') must differ from target alias ('${targetAlias}').`); + } @@ - if (!targetAlias) { - throw new Error(`Source expression does not have an alias. Please provide an alias for the merge target.`); - } + if (!targetAlias) { + throw new Error(`Source expression must have an explicit alias for MERGE.`); + } + if (targetAlias === normalized.sourceAlias) { + throw new Error(`sourceAlias ('${normalized.sourceAlias}') must differ from target alias ('${targetAlias}').`); + }Also applies to: 287-289, 321-323
682-684: Be explicit about CTEDisabler side‑effect.Assign the returned query to make the mutation explicit and future‑proof if CTEDisabler switches to immutable behavior.
Apply this diff:
- const cteDisabler = new CTEDisabler(); - cteDisabler.execute(selectQuery); + const cteDisabler = new CTEDisabler(); + selectQuery = cteDisabler.execute(selectQuery) as SimpleSelectQuery;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/core/src/models/BinarySelectQuery.ts(2 hunks)packages/core/src/models/SelectQuery.ts(3 hunks)packages/core/src/models/SimpleSelectQuery.ts(2 hunks)packages/core/src/models/ValuesQuery.ts(2 hunks)packages/core/src/transformers/InsertQuerySelectValuesConverter.ts(1 hunks)packages/core/src/transformers/QueryBuilder.ts(3 hunks)packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts(1 hunks)packages/core/tests/transformers/buildDeleteQuery.test.ts(1 hunks)packages/core/tests/transformers/buildInsertQuery.test.ts(1 hunks)packages/core/tests/transformers/buildMergeQuery.test.ts(1 hunks)packages/core/tests/transformers/buildUpdateQuery.test.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
packages/core/**/*.{ts,tsx}: Always compile and fix TypeScript errors before running tests
Source code comments must be written in English only
For model-driven mappings (mapping.typeInfo and mapping.structure), use convertModelDrivenMapping
Use imports from 'rawsql-ts' rather than local relative paths for library usage
Files:
packages/core/tests/transformers/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/src/transformers/InsertQuerySelectValuesConverter.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/src/models/SelectQuery.tspackages/core/src/models/BinarySelectQuery.tspackages/core/src/models/SimpleSelectQuery.tspackages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/src/transformers/QueryBuilder.tspackages/core/src/models/ValuesQuery.ts
packages/core/**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Tests are specs — never change expected values without user consultation
Files:
packages/core/tests/transformers/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/tests/transformers/buildDeleteQuery.test.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Remove console.log statements before commit
Files:
packages/core/tests/transformers/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/src/transformers/InsertQuerySelectValuesConverter.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/src/models/SelectQuery.tspackages/core/src/models/BinarySelectQuery.tspackages/core/src/models/SimpleSelectQuery.tspackages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/src/transformers/QueryBuilder.tspackages/core/src/models/ValuesQuery.ts
packages/core/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Use SqlFormatter for SQL comparison tests
Files:
packages/core/tests/transformers/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/tests/transformers/buildDeleteQuery.test.ts
🧠 Learnings (1)
📚 Learning: 2025-10-18T02:29:34.428Z
Learnt from: CR
PR: mk3008/rawsql-ts#0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-10-18T02:29:34.428Z
Learning: Applies to packages/core/**/*.{test,spec}.{ts,tsx} : Use SqlFormatter for SQL comparison tests
Applied to files:
packages/core/tests/transformers/buildInsertQuery.test.tspackages/core/tests/transformers/buildUpdateQuery.test.ts
🧬 Code graph analysis (11)
packages/core/tests/transformers/buildMergeQuery.test.ts (5)
packages/core/src/parsers/SelectQueryParser.ts (1)
SelectQueryParser(31-669)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)packages/core/src/models/MergeQuery.ts (4)
MergeUpdateAction(11-21)MergeInsertAction(33-51)MergeDoNothingAction(53-55)MergeDeleteAction(23-31)packages/core/src/models/ValueComponent.ts (2)
ColumnReference(47-82)ValueList(38-45)
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (2)
packages/core/src/parsers/InsertQueryParser.ts (1)
InsertQueryParser(14-106)packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (1)
InsertQuerySelectValuesConverter(12-125)
packages/core/tests/transformers/buildInsertQuery.test.ts (3)
packages/core/src/parsers/SelectQueryParser.ts (1)
SelectQueryParser(31-669)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (5)
packages/core/src/models/ValuesQuery.ts (1)
ValuesQuery(23-70)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/models/Clause.ts (3)
SelectItem(6-15)SelectClause(17-28)identifier(170-172)packages/core/src/models/BinarySelectQuery.ts (1)
BinarySelectQuery(36-216)packages/core/src/models/ValueComponent.ts (2)
ValueComponent(5-27)TupleExpression(396-403)
packages/core/tests/transformers/buildUpdateQuery.test.ts (2)
packages/core/src/parsers/SelectQueryParser.ts (1)
SelectQueryParser(31-669)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/src/models/SelectQuery.ts (2)
packages/core/src/models/ValueComponent.ts (3)
name(120-122)name(383-385)SqlParameterValue(223-223)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)
packages/core/src/models/BinarySelectQuery.ts (1)
packages/core/src/models/SelectQuery.ts (9)
BinarySelectQuery(100-100)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)
packages/core/src/models/SimpleSelectQuery.ts (2)
packages/core/src/models/SelectQuery.ts (9)
SelectQuery(88-97)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/tests/transformers/buildDeleteQuery.test.ts (2)
packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/src/transformers/QueryBuilder.ts (9)
packages/core/src/models/SelectQuery.ts (9)
SimpleSelectQuery(99-99)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/parsers/SourceExpressionParser.ts (1)
SourceExpressionParser(7-72)packages/core/src/models/Clause.ts (10)
InsertClause(576-589)UpdateClause(541-557)SetClauseItem(497-539)SetClause(476-483)FromClause(306-336)WhereClause(49-56)DeleteClause(559-569)UsingClause(338-348)SelectItem(6-15)WithClause(370-381)packages/core/src/models/MergeQuery.ts (5)
MergeWhenClause(57-69)MergeUpdateAction(11-21)MergeDeleteAction(23-31)MergeDoNothingAction(53-55)MergeInsertAction(33-51)packages/core/src/models/ValueComponent.ts (3)
ValueList(38-45)ColumnReference(47-82)BinaryExpression(198-209)packages/core/src/transformers/SelectValueCollector.ts (1)
SelectValueCollector(13-269)packages/core/src/transformers/CTECollector.ts (1)
CTECollector(25-501)packages/core/src/transformers/CTEDisabler.ts (1)
CTEDisabler(25-443)
packages/core/src/models/ValuesQuery.ts (2)
packages/core/src/models/SelectQuery.ts (9)
SimpleSelectQuery(99-99)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
🔇 Additional comments (11)
packages/core/tests/transformers/buildInsertQuery.test.ts (1)
8-20: LGTM: solid coverage for INSERT conversions and CTE hoisting.
- CTE hoist + options path, inferred columns, optional column dropping, explicit list filtering, and no-overlap error are all asserted precisely with SqlFormatter output. Good use of exact-equality for formatter-stable strings.
Also applies to: 22-29, 31-41, 43-53, 55-62
packages/core/tests/transformers/buildDeleteQuery.test.ts (1)
8-19: LGTM: DELETE with PKs and additional match columns is correctly asserted.USING source aliasing and equality predicates look right; strings match formatter output.
Also applies to: 21-33
packages/core/tests/transformers/buildUpdateQuery.test.ts (1)
8-19: LGTM: comprehensive UPDATE coverage (CTE, aliasing, composite PKs, column selection, errors).Legacy signature, options path, and error cases are well exercised with precise SqlFormatter assertions.
Also applies to: 21-33, 35-45, 47-58, 60-75, 77-89, 91-103, 105-114, 116-125
packages/core/tests/transformers/buildMergeQuery.test.ts (1)
9-41: LGTM: MERGE actions and column planning validated at AST level with good breadth.Aliases, action kinds, update/set items, insert columns/values, and error paths are covered.
Also applies to: 43-64, 66-82, 84-100, 102-111, 113-124
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (1)
18-26: LGTM: converter round‑trip and error coverage look solid with normalized SQL checks.Factory helper keeps the formatter usage tidy; assertions are precise.
Also applies to: 28-36, 38-47, 49-54, 56-61
packages/core/src/models/SimpleSelectQuery.ts (1)
129-131: UNION ALL helper looks goodConsistent with existing toBinaryQuery delegation. No issues.
packages/core/src/models/ValuesQuery.ts (1)
41-59: Delegations LGTMUsing toSimpleQuery() as a funnel point is clean and consistent.
packages/core/src/models/BinarySelectQuery.ts (2)
168-171: exceptAllRaw helper LGTMMatches the pattern used by other raw helpers.
173-187: New conversion helpers LGTMDelegation to toSimpleQuery() keeps behavior consistent across query variants.
packages/core/src/models/SelectQuery.ts (1)
88-97: Interface extension LGTMUnified toXQuery surface across query variants is clear and consistent.
packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (1)
52-57: No issues found—code uses correct constructor keyThe verification confirms the constructor accepts
returningas the parameter key (line 24 of InsertQuery.ts), and the code correctly passesreturning: insertQuery.returningClause. The field assignmentthis.returningClause = params.returning ?? null;(line 30) handles the mapping internally.
…rs and converters
…narySelectQuery, SimpleSelectQuery, and ValuesQuery; enhance QueryBuilder with conversion methods; update SqlPrintTokenParser for new query types
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/transformers/QueryBuilder.ts (1)
543-564: Consider clarifying the matchColumns logic.The logic at line 552 filters explicit columns to exclude primary keys, then lines 556-557 add primary keys separately. While correct, the two-step approach (exclude PKs from matchColumns, then add PKs to requiredColumns) could be clearer with a brief comment explaining that explicit columns provide additional matching criteria beyond the always-required primary keys.
Consider adding a clarifying comment:
let matchColumns: string[] = []; if (explicitColumns && explicitColumns.length > 0) { const normalized = QueryBuilder.normalizeColumnArray(explicitColumns); const preferred = new Set(normalized); + // Explicit columns add extra match criteria; PKs are always included separately matchColumns = selectColumns.filter(name => preferred.has(name) && !primaryKeySet.has(name)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/core/src/index.ts(1 hunks)packages/core/src/models/BinarySelectQuery.ts(2 hunks)packages/core/src/models/SimpleSelectQuery.ts(2 hunks)packages/core/src/models/SqlPrintToken.ts(1 hunks)packages/core/src/models/ValuesQuery.ts(2 hunks)packages/core/src/parsers/SqlPrintTokenParser.ts(3 hunks)packages/core/src/transformers/QueryBuilder.ts(3 hunks)packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts(1 hunks)packages/core/tests/transformers/buildMergeQuery.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/models/ValuesQuery.ts
- packages/core/src/models/SimpleSelectQuery.ts
- packages/core/tests/transformers/buildMergeQuery.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
packages/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
packages/core/**/*.{ts,tsx}: Always compile and fix TypeScript errors before running tests
Source code comments must be written in English only
For model-driven mappings (mapping.typeInfo and mapping.structure), use convertModelDrivenMapping
Use imports from 'rawsql-ts' rather than local relative paths for library usage
Files:
packages/core/src/index.tspackages/core/src/models/SqlPrintToken.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/src/transformers/QueryBuilder.tspackages/core/src/parsers/SqlPrintTokenParser.tspackages/core/src/models/BinarySelectQuery.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Remove console.log statements before commit
Files:
packages/core/src/index.tspackages/core/src/models/SqlPrintToken.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/src/transformers/QueryBuilder.tspackages/core/src/parsers/SqlPrintTokenParser.tspackages/core/src/models/BinarySelectQuery.ts
packages/core/**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Tests are specs — never change expected values without user consultation
Files:
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts
packages/core/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Use SqlFormatter for SQL comparison tests
Files:
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts
🧠 Learnings (1)
📚 Learning: 2025-10-18T02:29:34.428Z
Learnt from: CR
PR: mk3008/rawsql-ts#0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-10-18T02:29:34.428Z
Learning: Applies to packages/core/**/*.{test,spec}.{ts,tsx} : Use SqlFormatter for SQL comparison tests
Applied to files:
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts
🧬 Code graph analysis (4)
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (3)
packages/core/src/parsers/InsertQueryParser.ts (1)
InsertQueryParser(14-106)packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (1)
InsertQuerySelectValuesConverter(13-126)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(20-727)
packages/core/src/transformers/QueryBuilder.ts (9)
packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-753)packages/core/src/models/SelectQuery.ts (9)
SimpleSelectQuery(99-99)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)packages/core/src/parsers/SourceExpressionParser.ts (1)
SourceExpressionParser(7-72)packages/core/src/models/Clause.ts (10)
InsertClause(576-589)UpdateClause(541-557)SetClauseItem(497-539)SetClause(476-483)FromClause(306-336)WhereClause(49-56)DeleteClause(559-569)UsingClause(338-348)SelectItem(6-15)WithClause(370-381)packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (1)
InsertQuerySelectValuesConverter(13-126)packages/core/src/models/ValueComponent.ts (3)
ValueList(38-45)ColumnReference(47-82)BinaryExpression(198-209)packages/core/src/transformers/SelectValueCollector.ts (1)
SelectValueCollector(13-269)packages/core/src/transformers/CTECollector.ts (1)
CTECollector(25-501)packages/core/src/transformers/CTEDisabler.ts (1)
CTEDisabler(25-443)
packages/core/src/parsers/SqlPrintTokenParser.ts (2)
packages/core/src/models/MergeQuery.ts (6)
MergeWhenClause(57-69)MergeUpdateAction(11-21)MergeDeleteAction(23-31)MergeInsertAction(33-51)MergeDoNothingAction(53-55)MergeMatchType(5-5)packages/core/src/models/SqlPrintToken.ts (1)
SqlPrintToken(96-126)
packages/core/src/models/BinarySelectQuery.ts (1)
packages/core/src/models/SelectQuery.ts (8)
InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)
🔇 Additional comments (25)
packages/core/src/models/SqlPrintToken.ts (1)
86-91: LGTM! Clean addition of MERGE container types.The new enum members are well-named, follow the existing string-valued pattern, and are logically positioned to support MERGE query formatting.
packages/core/src/parsers/SqlPrintTokenParser.ts (8)
43-43: LGTM! Import statement is complete and correct.All necessary MERGE-related types are imported from the appropriate module.
314-319: LGTM! Handler registration is correct.All MERGE-related types are properly registered with their corresponding visitor methods, following the established pattern.
2393-2420: LGTM! MERGE query rendering is correct.The visitor properly constructs the MERGE INTO syntax with optional WITH clause, handles target/source/condition, and iterates through WHEN clauses with appropriate spacing.
2422-2439: LGTM! WHEN clause rendering is correct.Properly converts match types to keywords, handles optional AND conditions, and renders the THEN action with appropriate spacing.
2441-2466: LGTM! UPDATE and DELETE actions are correctly implemented.Both visitor methods properly render their respective actions with appropriate keywords, clauses, and spacing. Optional WHERE clause handling is consistent.
2468-2500: LGTM! INSERT action rendering is correct.The visitor properly handles:
- Optional column list with parentheses
- DEFAULT VALUES keyword
- VALUES clause with parentheses
- Appropriate spacing and separators
The implementation correctly uses early return for DEFAULT VALUES and follows the established token construction patterns.
2502-2504: LGTM! DO NOTHING action is correctly implemented.Simple and appropriate implementation. The underscore prefix for the unused parameter follows TypeScript conventions.
2506-2519: LGTM! Match type conversion is comprehensive.The switch statement correctly maps all MERGE match types to their SQL keywords with a safe default fallback. The keywords follow standard MERGE syntax.
packages/core/src/index.ts (1)
45-45: LGTM! Public API export is appropriate.The export correctly exposes the new
InsertQuerySelectValuesConverterclass, making it available to package consumers as part of the enhanced query conversion capabilities.packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (2)
1-8: LGTM! Test setup follows coding guidelines.The imports are appropriate and the formatter setup correctly uses
SqlFormatteras required by the coding guidelines for SQL comparison tests.Based on learnings.
9-101: Excellent test coverage!The test suite is comprehensive, covering:
- Bidirectional conversions (VALUES ↔ SELECT UNION ALL)
- Round-trip conversion without data loss
- Error scenarios (missing column lists, missing aliases, FROM clauses, tuple mismatches)
- Integration with QueryBuilder public API
All tests properly use
SqlFormatterfor normalized SQL comparisons and include clear, descriptive test names.packages/core/src/models/BinarySelectQuery.ts (2)
1-19: LGTM! Imports are properly structured.Type-only imports for conversion options and query types are correctly used, which aids tree-shaking and prevents circular dependency issues.
171-201: Clean delegation pattern for query conversions.The four new conversion methods (
toInsertQuery,toUpdateQuery,toDeleteQuery,toMergeQuery) follow a consistent pattern by delegating totoSimpleQuery()before applying the conversion. The JSDoc comments appropriately note that SELECT list reordering may occur, which is important for users to understand.packages/core/src/transformers/QueryBuilder.ts (11)
228-242: LGTM! Clean delegation to helper methods.The updated
buildInsertQueryproperly delegates to normalization and preparation helpers before constructing the InsertQuery. The comment clearly explains the flow: normalize options → prepare columns → extract CTEs → build query.
249-260: LGTM! Thin wrappers for converter methods.The
convertInsertValuesToSelectandconvertInsertSelectToValuesmethods cleanly delegate toInsertQuerySelectValuesConverter, providing a consistent QueryBuilder API surface for users.
265-295: LGTM! Well-structured UPDATE query builder.The method properly:
- Normalizes input options (backward compatibility maintained)
- Validates target alias presence
- Extracts CTEs for cleaner SQL
- Constructs SET items from prepared columns
- Builds equality predicates for the WHERE clause
Error messages are clear and guide users toward resolution.
300-321: LGTM! DELETE query builder follows consistent pattern.The implementation correctly:
- Normalizes options and prepares predicate columns
- Validates target alias requirement
- Extracts CTEs
- Builds USING clause with the source query
- Constructs WHERE predicate for matching rows
326-396: LGTM! Comprehensive MERGE query builder.The implementation handles the complexity of MERGE operations well:
- Prepares separate column sets for update/insert actions
- Validates target alias presence
- Constructs appropriate WHEN clauses based on action types (update/delete/doNothing)
- Validates that at least one action is generated
- Provides clear error messages for missing columns
The logic for handling the three MERGE scenarios (matched, not_matched, not_matched_by_source) is correct and complete.
398-461: LGTM! Robust input normalization.The normalization helper methods properly:
- Maintain backward compatibility with legacy signatures (especially
normalizeUpdateOptions)- Provide sensible defaults (
sourceAlias: 'src')- Validate inputs (
normalizeColumnArrayensures non-empty arrays)- Return consistent, fully-normalized option objects
This defensive approach prevents downstream errors and provides clear validation messages.
496-511: LGTM! INSERT column preparation preserves explicit ordering.The implementation correctly:
- Validates all explicit columns exist in the select list (lines 501-503)
- Deduplicates while preserving order (line 500)
- Reorders the SELECT clause to match caller-provided column order (line 505)
- Falls back to select list order when explicit columns not provided
This addresses the past review feedback about honoring caller-provided column order.
513-541: LGTM! UPDATE column preparation with proper PK handling.The implementation correctly:
- Ensures primary keys exist in the select list (line 515)
- Filters out PKs from update candidates (line 518)
- Preserves explicit column order when provided (lines 521-528)
- Validates explicit columns aren't PKs and exist in candidates (lines 524-527)
- Reorders SELECT with PKs first, then update columns (lines 533-537)
This addresses the past review feedback about explicit column ordering.
566-630: LGTM! MERGE column preparation with comprehensive validation.The implementation correctly:
- Validates all explicit columns (update and insert) exist in select list (lines 585-587, 600-602)
- Preserves explicit column order for both update and insert actions (lines 589, 604)
- Deduplicates while maintaining order (lines 584, 599)
- Builds a comprehensive desiredOrder that prioritizes PKs, then action-specific columns (lines 610-615)
- Returns separate column lists based on the actual action types (lines 619-629)
This addresses the past review feedback about preserving explicit ordering in MERGE operations.
632-663: LGTM! SELECT clause reordering with validation.The implementation correctly:
- Maps existing items by name for lookup (lines 633-642)
- Rebuilds items in the desired order (lines 644-656)
- Validates each requested column exists (lines 651-653)
- Deduplicates using a seen Set (lines 645, 647-650, 655)
- Directly mutates the selectClause.items (line 662), which is intentional for in-place reordering
665-726: LGTM! Clean utility methods.The utility methods are well-implemented:
getSelectItemName: Extracts names from SelectItem with proper fallback logicensureSelectClauseSize: Defensive validation with clear error messageextractWithClause: Clean CTE extraction and removal using CTECollector/CTEDisablerbuildEqualityPredicate: Correctly chains equality comparisons with ANDtoColumnReference: Simple factory for consistencymergeUniqueColumns: Order-preserving deduplicationAll methods have clear, single responsibilities and proper error handling.
1 participant
Summary by CodeRabbit
New Features
Tests